Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't run preg_match() on null values #39927

Merged
merged 1 commit into from
Apr 1, 2022

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Mar 31, 2022

PHP 8.1 has deprecated passing null as the second parameter to preg_match(), generating the following Deprecated notice: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated.

What?

Avoids a PHP Deprecated notice

Why?

PHP 8.1 has deprecated passing null values to preg_match(), which is the default condition here.

How?

Check the $block_gap is a non-falsey value before processing it.
This style is already used for this variable value further down the stack:

$gap_style = $gap_value && ! $should_skip_gap_serialization ? $gap_value : 'var( --wp--style--block-gap )';

The changes in this PR could also be written as:

-		$gap_value = $gap_value && preg_match( '%[\\\(&=}]|/\*%', $gap_value ) ? null : $gap_value;
+		$gap_value = ! $gap_value || preg_match( '%[\\\(&=}]|/\*%', $gap_value ) ? null : $gap_value;

both are logically the same here (although different around a 0 or false, which is not supported here due to the above mentioned code), as it's only checking to see if it contains invalid characters.

Testing Instructions

Unknown.

Have a block that does not contain the blockGap key: _wp_array_get( $block, array( 'attrs', 'style', 'spacing', 'blockGap' ) ).

Screenshots or screencast

PHP 8.1 has deprecated passing null as the second parameter to `preg_match()`, generating the following Deprecated notice: `preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated`.
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@aristath
Copy link
Member

aristath commented Apr 1, 2022

The failing e2e test here is unrelated to this PR, and this is an essential fix for PHP8.x compatibility so I'll go ahead and merge this.
The code looks good, makes perfect sense, and works as expected.

@aristath aristath merged commit 689416b into trunk Apr 1, 2022
@aristath aristath deleted the dd32/patch-php81-nullable-to-preg_match branch April 1, 2022 09:19
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Apr 1, 2022
@ramonjd
Copy link
Member

ramonjd commented Apr 5, 2022

Thanks for catching that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants